-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
get rid of nuget #77
get rid of nuget #77
Conversation
Speed up 4x-8x
@@ -162,5 +162,5 @@ function handleSquirrelEvent() { | |||
You can get debug messages from this package by running with the environment variable `DEBUG=electron-windows-installer:main` e.g. | |||
|
|||
``` | |||
DEBUG=electron-windows-installer:main node tasks/electron-winstaller.js | |||
DEBUG=electron-windows-installer node tasks/electron-winstaller.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to debug — you need all categories in any case.
The issue — if you use different categories, you cannot get time information (debug measures time per category).
This almost certainly has correctness issues because you hard-code the list of extensions in the content types file. Why do we care about the speed that packaging happens again? |
@paulcbetts Thanks for review.
Why it is important? Where Squirrel.Windows uses it? If it is required, I think, we should fix it and don't require?
Because 10 minutes (or hang) is not acceptable. electron-userland/electron-builder#351 — "in at 99% CPU usage for about ten minutes.". And here no sense to fix nuget or Squirrel.Windows — it is very easy to write cross-platform solution to pack (I don't say that node is perfect — but .net platform is definitely not suitable here). |
NuGet uses it, Squirrel.Windows doesn't really care In general, I'd rather favor correctness over speed - this PR already has issues with generating correctly formed NuGet packages and I don't really want to be in the situation where a bunch of apps now report "My installer is broken and I just shipped it to 12030232x users!" We cannot ship broken updates, and determining whether this fix is correct in 100% of scenarios is not trivial. Sorry. |
If someone can give me a repro of the 10-minute builds, I can look into why NuGet.exe is taking so long |
@ArekSredzki Is it possible to attach your project? It seems, any project without asar and a lot of files is an example, but it will be better to have real app. @paulcbetts Both problems — on nuget and Squirrel.windows — in the zip library, I guess (see electron-userland/electron-builder#351 (comment) log) |
@paulcbetts Also, nuget is a plain zip file with a some predefined structure.
How? Why it is important? App files are using only by Electron, for nuget it can be safely be a just If it is the only problem, I don't think it is even has minor priority. Because as result we don't depend on win-only tool.
We build setup.exe for Squirrel.Windows, not for nuget, only for Squirrel.Windows. It is cool that Squirrel.Windows doesn't use this bad-smell and questionable approach to define Content-Types. So, why it is a problem? BTW, actually, there is already alternative implementation of nuget pack — http://docs.octopusdeploy.com/display/OD/Using+Octo.exe And in any case we don't try to reimplement nuget pack — we just need to provide data to Squirrel.Windows. https://github.com/jordansissel/fpm allows us just provide directory mappings. It will be an ideal solution here. But currently Squirrel.Windows requires questionable nuget file. I understand the reasons — Squirrel.Windows was developer for windows apps. But Electron app is just a bunch of files (no deps). Also — electron-builder tests check resulting nupkg (full), so, if content is the same, I am sure that we will not get "My installer is broken and I just shipped it to 12030232x users!" Please note — again, we don't build nuget package, we build setup.exe and update files or Squirrel.Windows (nuget or delta — it is all not about nuget, but about Squirrel.Windows (and ideally, 7z compression should be used (I have exact number for mac— 28 vs 40 MB)). |
So sad to see this closed. Our Mono builds are extremely slow. It's 32 minutes and counting... |
Speed up 4x-8x (only nuget part, Squirrel.Windows is another part of story).
Related issues: #55, electron-userland/electron-builder#351
Works in electron-builder (it is the reason why I created PR only now).
This PR blocks PR about code signing on OS X (it is fixed in my fork also).